Recode cob_decimal_pow function - Fix for #924, #925, #989 - add test cases for power operator #182

denishug · 2024-09-25T21:08:10Z

No description provided.

- Fixe #924, #925, #989 - add tests case for power operator

libcob/intrinsic.c

engboris · 2024-09-26T17:34:24Z

Hello @denishug and thank you for your contribution.

It is usually requested that contributors add an entry to ChangeLog files when modifying the source code. Here, I believe libcob/ChangeLog must be modified. There is a strict formatting (number of spaces, difference between space and tab).

Example in our PR #191 (merged)

Update Changelog

libcob/ChangeLog

engboris · 2024-10-21T12:23:31Z

libcob/intrinsic.c

+	/* Fix  #925 : Avoid GMPLIB CRASH */
+
+	mpf_set(cob_mpft3,cob_mpft) ;
+	if ( sign_nbr == -1) {


Remove space before condition

engboris · 2024-10-21T12:24:32Z

libcob/intrinsic.c

+
+ Process_case_0:
+	if (sign_nbr == 0) {
+		if ( sign_exp == 1) {


Remove space before condition

libcob/intrinsic.c

GitMensch

Thanks for this nice piece of work. I think the Changelog entries could be improved and there are some code parts + formatting, but overall quite good!

ChangeLog

libcob/ChangeLog

GitMensch · 2024-10-21T12:24:11Z

tests/testsuite.src/run_fundamental.at

+AT_CHECK([$COMPILE prog.cob], [0], [], [])
+AT_CHECK([$COBCRUN_DIRECT ./prog], [0], [], [])
+AT_CLEANUP
+


minor beautify: please separate test cases with a double empty line - also above

GitMensch · 2024-10-21T12:26:12Z

tests/testsuite.src/run_fundamental.at

+AT_CLEANUP
+
+AT_SETUP([Power size error and limits cases 2])
+AT_KEYWORDS([POWER])


Words that are in AT_SETUP are automatically added to the keyword list. Replacing it with the main statement - here COMPUTE - would be useful (also applies above).

GitMensch · 2024-10-21T12:28:32Z

tests/testsuite.src/run_fundamental.at

please add an m4 comment about which tests are for which bug report - this provides more context and is also useful in the future

GitMensch · 2024-10-21T12:31:18Z

libcob/intrinsic.c

+		break;
+
+	case 0:
+		goto Process_case_0;


there is a single case for this - please inline instead of using goto

GitMensch · 2024-10-21T12:32:51Z

libcob/intrinsic.c

-		mpz_set_ui (pd1->value, 1UL);
-		pd1->scale = 0;
-		return;
+		goto Compute_Power;


please reorder that case to have zero first, this can then return while the fall-through of 1/-1 is easier to follow and we don't need the Compute_Power label any more

GitMensch · 2024-10-21T12:33:38Z

libcob/intrinsic.c

+	sign_nbr = mpz_sgn(pd1->value);
+	sign_exp = mpz_sgn(pd2->value);
+
+	power_case = sign_nbr * sign_exp;
+
+	cob_trim_decimal(pd2);
+	cob_trim_decimal(pd1);


Can you please reformat to use a space before the parenthesis (also applies to other places)?

libcob/intrinsic.c

GitMensch · 2025-01-19T12:37:02Z

I've fixed the merge conflicts in the Changelogs, resolving all open issues on these.
There are still (minor) open issues to the other two changed files with a note "done", but not push. Please git pull to get the current changes in (no need for Changelog updates as long as these are all changes to your previous ones), then review your changes / check those in and push them back.

Otherwise I can also handle that when doing the commit upstream - just give me a note how we want to handle that.

run_fundametal.at : add #1020 ticket test

Recode cob_decimal_pow function

85b1770

- Fixe #924, #925, #989 - add tests case for power operator

engboris requested a review from GitMensch September 26, 2024 08:57

lefessan reviewed Sep 26, 2024

View reviewed changes

libcob/intrinsic.c Outdated Show resolved Hide resolved

engboris requested review from engboris and removed request for engboris September 26, 2024 17:22

denishug added 2 commits September 26, 2024 21:49

20240925 Correct libcob/intrinsic.c code to compile on mac os

ba09a87

Update Changelog

2024-09-27 Fix C style comment in intrinsic.c (cob_decimal_pow function)

6ce83a5

engboris reviewed Oct 21, 2024

View reviewed changes

libcob/ChangeLog Outdated Show resolved Hide resolved

engboris reviewed Oct 21, 2024

View reviewed changes

libcob/ChangeLog Outdated Show resolved Hide resolved

engboris reviewed Oct 21, 2024

View reviewed changes

libcob/intrinsic.c Outdated Show resolved Hide resolved

GitMensch requested changes Oct 21, 2024

View reviewed changes

engboris changed the title ~~Recode cob_decimal_pow function - Fixe #924, #925, #989 - add tests case for power operator~~ Recode cob_decimal_pow function - Fix #924, #925, #989 - add tests case for power operator Oct 21, 2024

engboris changed the title ~~Recode cob_decimal_pow function - Fix #924, #925, #989 - add tests case for power operator~~ Recode cob_decimal_pow function - Fix for #924, #925, #989 - add tests case for power operator Oct 21, 2024

engboris changed the title ~~Recode cob_decimal_pow function - Fix for #924, #925, #989 - add tests case for power operator~~ Recode cob_decimal_pow function - Fix for #924, #925, #989 - add test cases for power operator Oct 21, 2024

cob_decimal_pow : fix typo and add #1020 ticket fix

14e8b5f

run_fundametal.at : add #1020 ticket test

denishug force-pushed the gcos4gnucobol-3.x branch from d262205 to 14e8b5f Compare January 26, 2025 12:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Recode cob_decimal_pow function - Fix for #924, #925, #989 - add test cases for power operator #182

Recode cob_decimal_pow function - Fix for #924, #925, #989 - add test cases for power operator #182

denishug commented Sep 25, 2024

engboris commented Sep 26, 2024

engboris Oct 21, 2024

engboris Oct 21, 2024

denishug Jan 18, 2025

GitMensch left a comment

GitMensch Oct 21, 2024

GitMensch Oct 21, 2024

GitMensch Oct 21, 2024

GitMensch Oct 21, 2024

GitMensch Oct 21, 2024

GitMensch Oct 21, 2024

denishug Dec 28, 2024

GitMensch commented Jan 19, 2025

Recode cob_decimal_pow function - Fix for #924, #925, #989 - add test cases for power operator #182

Are you sure you want to change the base?

Recode cob_decimal_pow function - Fix for #924, #925, #989 - add test cases for power operator #182

Conversation

denishug commented Sep 25, 2024

engboris commented Sep 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GitMensch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GitMensch commented Jan 19, 2025